Skip to content

Comments

fix rules for image-based gadgets#24

Merged
amirmalka merged 11 commits intomainfrom
image-based
Nov 25, 2025
Merged

fix rules for image-based gadgets#24
amirmalka merged 11 commits intomainfrom
image-based

Conversation

@matthyx
Copy link
Contributor

@matthyx matthyx commented Oct 16, 2025

Note

Migrate all rules and tests to a unified event.* schema, generate a combined CRD, and update Go/tooling/dependencies.

  • Rules/CRD:
    • Migrate all rule YAMLs to unified event.* fields (uniqueId, ruleExpression, eventType), add isTriggerAlert and MITRE metadata.
    • Add/refresh many rule definitions and generate consolidated rules-crd.yaml.
  • Tests:
    • Refactor to use utils.StructEvent instead of gadget-specific types; align assertions with new message/ID formats.
  • CI:
    • Set Go toolchain via GOTOOLCHAIN=go1.25.0+auto and add steps to generate and upload Rules CRD artifact.
  • Dependencies/Build:
    • Bump to Go 1.25 and update numerous modules; switch inspektor-gadget to a replacement fork.
  • Docs:
    • Update README to reflect new field names and schema conventions.

Written by Cursor Bugbot for commit 6adb258. This will update automatically on new commits. Configure here.

matthyx and others added 4 commits October 16, 2025 08:49
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Amit Schendel <amitschendel@gmail.com>
Signed-off-by: Amit Schendel <amitschendel@gmail.com>
cursor[bot]

This comment was marked as outdated.

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
amitschendel and others added 2 commits October 20, 2025 09:22
Signed-off-by: Amit Schendel <amitschendel@gmail.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Amit Schendel <amitschendel@gmail.com>
@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Container Name Missing in Cache

The ContainerInfo struct's Name field is commented out in rule_test.go's object cache setup. This prevents the container name from being properly set, leaving the cached container with an empty name. As a result, container matching may fail during rule evaluation.

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Network Event Struct Mismatch Causes CEL Errors

Network event tests define DstEndpoint as a nested eventtypes.L3Endpoint struct, but the YAML rules expect event.dstAddr as a direct string field. This structural mismatch causes type errors during CEL rule evaluation.

Additional Locations (3)

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Trailing Underscore in Unique ID Expression

The uniqueId expression for the R1004 rule includes an extra + '_' at the end, causing generated unique IDs to have an unnecessary trailing underscore.

Additional Locations (1)

Fix in Cursor Fix in Web

Signed-off-by: Amit Schendel <amitschendel@gmail.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Signed-off-by: Amit Schendel <amitschendel@gmail.com>
Signed-off-by: Amit Schendel <amitschendel@gmail.com>
objectcache.Container: {
{
Name: tt.event.Event.K8s.BasicK8sMetadata.ContainerName,
//Name: tt.event.Event.K8s.BasicK8sMetadata.ContainerName,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Refactoring Left Container Name Uninitialized

The Name field initialization is commented out in the ContainerInfo struct, leaving an empty struct in the array. This appears to be incomplete refactoring where the developer commented out the old field access pattern but didn't replace it with the new one. The container name should be set to tt.event.Container to properly initialize the test data.

Fix in Cursor Fix in Web

(event.path.startsWith('/var/run/secrets/kubernetes.io/serviceaccount') && event.path.endsWith('/token')) ||
(event.path.startsWith('/run/secrets/eks.amazonaws.com/serviceaccount') && event.path.endsWith('/token')) ||
(event.path.startsWith('/var/run/secrets/eks.amazonaws.com/serviceaccount') && event.path.endsWith('/token'))) &&
!ap.was_path_opened_with_suffix(event.containerId, '/token')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Broad Token Path Check Bypasses Security

The rule changed from checking if any of the 4 specific service account directory prefixes were accessed to checking if any path ending with /token was accessed. This creates a security bypass: if the application profile contains any file ending with /token (like /tmp/myapp/token), it will whitelist access to the actual service account token at /run/secrets/kubernetes.io/serviceaccount/token. The old logic was more secure by specifically checking for service account directory prefixes rather than any path with the /token suffix.

Fix in Cursor Fix in Web

@amirmalka amirmalka merged commit 27dfd4b into main Nov 25, 2025
4 checks passed
@matthyx matthyx moved this from WIP to To Archive in KS PRs tracking Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants